Skip to content

Finish agent-selection cleanup and restore merge validation - #28607

Merged
nwparker merged 5 commits into
mainfrom
fix/workspace-agent-clear-merge-validation
Oct 14, 2026
Merged

nwparker merged 5 commits into
mainfrom
fix/workspace-agent-clear-merge-validation

Conversation

@nwparker

@nwparker nwparker commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

Finish the agent-selection cleanup, preserve default tab styling on headless hosts, and require the complete PR checks before merging.

What Changed

Retire translated None labels and hints that no longer exist in the English catalog. Give the mobile check the distinct name Mobile verify; its internal job ID and steps remain unchanged. Add a regression test reserving verify for the full PR workflow. Read setup settings only when setup actually runs, so default-tab styling works on headless runtimes without a settings store. Replace a fixed 5 ms delay in the cookie-import concurrency test with waiting for the first write. Restore one option-mutation clock after concurrent fixes on main removed both duplicates.

Why

#28543 auto-merged when mobile verification passed while the full PR checks were still running. GitHub treats matching check names as the same required context. A unique mobile name prevents that early merge; retiring unused translations fixes the catalog failure the full check subsequently reported. Full-suite CI also caught the early settings read blocking headless tab styling and an unrelated concurrency test racing asynchronous file reads; both now have deterministic validation.

Linked Issue

Follow-up to #28543.

Visual Proof

The agent-selection screens and rendered setup-only workspace are shown in the visual proof on #28543. This follow-up restores existing headless default-tab behavior, retires unused labels, and fixes CI naming; it adds no screen or layout.

Testing

  • 18 workflow regression tests pass across three files.
  • Localization catalog verification passes for English and all five translated catalogs; full checks must pass before merge.
  • 27 provisioning, headless default-tab (local and SSH), and cookie-import concurrency tests pass across three files.
  • 108 agent-session tests pass after restoring the clock, including real host permission-option mutations, queued options, and stop handling.
  • Automation UI and remote protocol remain unchanged.

Review

Verified the provisioning fix against real headless runtime tests for local and SSH agent tab titles/colors; templates are still skipped for a cleared setup-only choice. The cookie test now proves the first import holds the lock before starting the second.

Checked that mobile job references use its unchanged job ID, and that only the full PR workflow now publishes verify on pull requests. The full PR gate waits for tests, static analysis/typecheck, and packaging.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer material.

Checklist

  • Small and focused
  • Before/after, mechanism, and rationale explained
  • Visual proof applicability explained
  • Reviewed correctness and cross-platform impact
  • Full CI passes on 4e9ede7: all five Node 24 test shards, static analysis/typecheck/localization, relay integration, cross-version wire compatibility, Linux/Windows packaging, mobile verification, persistence, and the complete PR verify gate.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b2fa1fb3-b264-422e-87d5-a1570b6a04e3
📥 Commits

Reviewing files that changed from the base of the PR and between 9908fe4b949a064559e33d9f70be9d6211d0fcaa and 4e9ede7.

📒 Files selected for processing: 4
  • config/scripts/required-pr-check-name.test.mjs
  • src/main/browser/browser-cookie-import-concurrency.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-message-options.ts
  • src/main/runtime/runtime-worktree-terminal-provisioning.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The mobile workflow’s verify job now has the display name “Mobile verify.” A Vitest test checks which pull-request workflow provides the matching verify check. Spanish, French, Japanese, Korean, and Chinese locale files remove setup-only agent option labels and hints. The option-operation context exposes a now callback. Worktree terminal provisioning reads the launch-mode setting only when setup is configured. A cookie-import concurrency test waits for the first import’s write event instead of using a fixed delay.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 4e9ed

The mobile check is distinct from full PR verification, and the option clock, headless fallback, and import synchronization preserve their stated behavior. No actionable merge risk remains from these changes.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely summarizes the main changes: agent-selection cleanup and restoration of merge validation.
Description check Passed The description is complete and covers the required sections, including user-facing changes, rationale, linked issue, visual proof, testing, review notes, and checklist status. It also clearly states …
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments: 1
config/scripts/required-pr-check-name.test.mjs: 1

1-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Include .yaml workflows in the uniqueness scan.

.yaml is an accepted workflow format in this repository. The .yml-only filter skips a future .yaml pull-request workflow with a verify job, so the test can still pass while GitHub creates a duplicate verify check.

Suggested fix
-    .filter((file) => file.endsWith('.yml'))
+    .filter((file) => /\.ya?ml$/.test(file))

ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c57f385e-2b89-4bcb-b536-62a862ddacd9
📥 Commits

Reviewing files that changed from the base of the PR and between eb7f427 and 9908fe4b949a064559e33d9f70be9d6211d0fcaa.

📒 Files selected for processing: 7
  • .github/workflows/mobile.yml
  • config/scripts/required-pr-check-name.test.mjs
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

@nwparker nwparker changed the title Fix stale agent labels and ambiguous merge checks Finish agent-selection cleanup and restore merge validation Oct 14, 2026
@nwparker
nwparker force-pushed the fix/workspace-agent-clear-merge-validation branch from 4453786 to 02ebba7 Compare October 14, 2026 06:31
@nwparker

Copy link
Copy Markdown
Contributor Author

Addressed the workflow-extension review note in 4e9ede7: the uniqueness guard now scans both .yml and .yaml. The focused guard test passes. Final CI is running on that head.

@nwparker
nwparker merged commit b83a6f7 into main Oct 14, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant